Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit test for ensembl.io.genomio.genome_metadata.dump module #301

Merged
merged 17 commits into from
Mar 6, 2024

Conversation

JAlvarezJarreta
Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta commented Feb 29, 2024

Updated all methods (review docstrings, improve readability and reduce code complexity) and added unit tests for all of them.

In the case of get_genome_metadata() the rework whilst may not be as efficient as the previous code, I believe it is more readable and allows for further meta table checks in case the data found there does not match the expected format, e.g. is species meta key always has a subkey but there is an entry without, a ValueError is raised.

Added to __all__ one of the functions that was missing.

Copy link
Contributor

@MatBarba MatBarba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some questions about the code

@JAlvarezJarreta
Copy link
Contributor Author

JAlvarezJarreta commented Feb 29, 2024

@MatBarba asked for a re-review due to the addition of updated and unit test for filter_genome_meta() (messed up the branching/commits and was unable to push review updates in this PR without pushing this as well). Sorry!

@JAlvarezJarreta JAlvarezJarreta changed the title Add unit test for ensembl.io.genomio.genome_metadata.dump module (round 1) Add unit test for ensembl.io.genomio.genome_metadata.dump module Mar 4, 2024
Copy link
Contributor

@MatBarba MatBarba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. At some point we might want to test with test databases, but that part is pretty "simple" so the testing is clear.

@JAlvarezJarreta JAlvarezJarreta merged commit 486c1a6 into hackathon/feb24 Mar 6, 2024
1 check failed
@JAlvarezJarreta JAlvarezJarreta deleted the jalvarez/hack_feb24 branch March 6, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants